Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(learn): add article for publishing a typescript package #7279

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

JakobJingleheimer
Copy link
Member

@JakobJingleheimer JakobJingleheimer commented Nov 23, 2024

Description

Document the recommended way to publish a typescript package

Validation

Related Issues

nodejs/typescript#19

Depends on #7229

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run npm run format to ensure the code follows the style guide.
  • I have run npm run test to check if all tests are passing.
  • I have run npx turbo build to check if the website builds without errors.
  • I've covered new added functionality with unit tests if necessary.

Copy link

vercel bot commented Nov 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview Dec 30, 2024 0:04am

@AugustinMauroy
Copy link
Member

AugustinMauroy commented Dec 29, 2024

Jacob you should:

fail-fast: false # prevent a failure in one version cancelling other runs

steps:
- uses: actions/checkout@v4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these version refs will likely get outdated pretty fast; what’s the plan for keeping them updated?

"lint": "…",
"types:check": "tsc --noEmit"
},
"optionalDependencies": {
Copy link
Member

@ljharb ljharb Dec 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is incorrect; optional deps are runtime deps, and will always be installed - it just won’t fail the overarching install if it fails to compile. This will definitely install typescript locally, and for all consumers. There is no way to have optional dev deps, and “optional” wouldn’t be the same as it means for peerDependenciesMeta.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible via --omit, I just haven't got to that yet :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For an application, sure, but then doesn't that force consumers to use it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, why? As you say, it won't do anything magical on its own.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anything in optionalDependencies will automatically be installed for consumers precisely the same as if it's in dependencies.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes…? Exactly?

Copy link
Member

@ljharb ljharb Dec 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and why would typescript be a runtime dependency? it should only ever be a peerDependency of a package, at most, and a package that isn't directly about typescript shouldn't be requiring consumers to install typescript at all.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JakobJingleheimer Can we change it to devDependencies instead?

apps/site/pages/en/learn/typescript/publishing.md Outdated Show resolved Hide resolved
- name: Publish with provenance
env:
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
run: npm publish --access public --provenance
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this publishes with one factor, which is much less secure than publishing with 2fa; i strongly suggest node not in any way encourage doing this.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way to set up automated version publishing with 2FA? That would be ideal - automated and secure.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, not without an external server - until GitHub adds a native way for a workflow to pause and wait for user input to continue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oula, I don't agree with you 100%. First of all you can ask for the 2FA for the provenance depending on the NPM token used. Also I think Node.js should recommend packages with provenance for security reasons.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean - the only way to do two-factor is with two factors. Using an npm token is only ever a single factor.

As for provenance, I'm happy to discuss that separately, but whether it has value on its own or not is irrelevant - having 2FA is universally recognized as table stakes for security, which is why github and npm are making it required for everyone. They're not even planning to ever require provenance information, which should tell you about the relative security value of each.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, the way npm publish/automation tokens work, 2FA is bypassed entirely - that's my point.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "bypassed entirely", whilst technically possible, is not realistic. Yes, GitHub has the token and could do it. That possibility seems…remote. As you say, it is a trusted CI system. Aside from that, both sides are guarded by 2FA, and left or right are the only realistic access-points—when both actors are trusted, it seems reasonable to trust whatever they're doing, and provenance assures there has been no slight of hand / man-in-the-middle.

If we could have both, yeah, let's do it!

I don't know enough to authoritatively say what we should do since we apparently must choose one or the other, so I think we should defer to our own team of security experts (@marco-ippolito 👀).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub could be exploited, separate from whether one of the tens of thousands of Microsoft employees was a bad actor.

I’m suggesting node simply not recommend a publishing method rather than wade into a conversation that industry security experts don’t even agree on.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's give the security team a chance to weigh in. If we don't get a definitive answer, then let's come up with an alternative (I would be inclined to list both options and, possibly in an addendum, explain some of the key advantages and disadvantages—especially since this info was difficult for even us to gather).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally speaking I'd omit giving an advice on this matter completely and leave the user to do their own research on how to publish on npm since its out of the scope of this guide. Ssince there is no out of the box alternative I'm ok with suggesting 1F (which we use on many of our @nodejs packages)

@JakobJingleheimer

This comment was marked as outdated.

Co-authored-by: Augustin Mauroy <[email protected]>
Signed-off-by: Jacob Smith <[email protected]>

- Node runs TypeScript code via a process called "[type stripping](https://nodejs.org/api/typescript.html#type-stripping)", wherein node (via [Amaro](https://github.com/nodejs/amaro)) removes TypeScript-specific syntax, leaving behind vanilla JavaScript (which node already understands). This behaviour is enabled by default of node version 23.6.0.

- Node does **not** strip types in `node_modules` because it can cause significant performance issues for the official TypeScript compiler (`tsc`), so the TypeScript maintainers would like to discourage people publishing raw TypeScript, at least for now.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add that it slow down the VSCode language server as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the same reason, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes exactly

},
"optionalDependencies": {
"typescript": "^5.7.2"
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this an optionalDependency?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the reasons discussed in this PR 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
```

Pro-tip: The TypeScript executable (`tsc`) is likely used only in CI. Avoid bloating your local node_modules (where you probably won't use it) by adding [`--omit="optional"`](https://docs.npmjs.com/cli/v11/commands/npm-install#omit) when you run `npm install` locally: `npm install --omit="optional"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually not something I would recommend. Move typescript to a devDependency

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would you not recommend?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, optional is something not needed for the project. Dev is used for dev part and classic for runtime. So to transpile we need to put it in dev

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly that. Otherwise ts will be installed every time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree we should advice it as devDependency

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The people have spoken 😅👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants